fix(api): handle empty string POSTHOG_KEY from Docker Compose - #1728
Conversation
Docker Compose sets POSTHOG_KEY to an empty string (not undefined) when the variable is unset on the host. The nullish coalescing operator only catches null/undefined, so the empty string was passed to the PostHog constructor which throws because it requires a non-empty key. Normalize empty strings to undefined before applying the fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis change introduces a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the PostHog constructor would throw an assertion error when POSTHOG_KEY or POSTHOG_HOST environment variables are set to empty strings by Docker Compose. The fix adds a nonEmpty() helper function that normalizes empty strings to undefined, allowing the nullish coalescing operator to properly fall back to placeholder values.
Changes:
- Added
nonEmpty()helper to convert empty strings toundefined - Refactored PostHog initialization to use the helper before applying fallback values
- Improved code comments explaining the Docker Compose empty string behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function nonEmpty(value: string | undefined): string | undefined { | ||
| if (!value) { | ||
| return undefined; | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
The nonEmpty function uses a falsy check which correctly handles empty strings, but it won't catch whitespace-only strings like " " or "\t". Consider using .trim() before the check to ensure whitespace-only strings are also normalized to undefined. For example: if (!value?.trim()) { return undefined; }
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/posthog.ts (1)
6-11:nonEmptycan be simplified to a one-linerThe four-line body is equivalent to
return value || undefined.♻️ Suggested simplification
function nonEmpty(value: string | undefined): string | undefined { - if (!value) { - return undefined; - } - return value; + return value || undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/posthog.ts` around lines 6 - 11, The function nonEmpty currently has a multi-line body; simplify it by returning the expression value || undefined directly in the function body so nonEmpty(value: string | undefined): string | undefined { return value || undefined; }, keeping the same signature and behavior and referencing the existing function name nonEmpty and its parameter value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/posthog.ts`:
- Around line 6-11: The function nonEmpty currently has a multi-line body;
simplify it by returning the expression value || undefined directly in the
function body so nonEmpty(value: string | undefined): string | undefined {
return value || undefined; }, keeping the same signature and behavior and
referencing the existing function name nonEmpty and its parameter value.
Summary
POSTHOG_KEYto an empty string (notundefined) when the variable is unset on the host??) only catchesnull/undefined, so the empty string""was passed to the PostHog constructorassert()throws because it requires a non-empty API key, even whendisabled: trueis setnonEmpty()helper to normalize empty strings toundefinedbefore the??fallbackTest plan
POSTHOG_KEYis properly set🤖 Generated with Claude Code
Summary by CodeRabbit